-
Notifications
You must be signed in to change notification settings - Fork 5
feat: Improve visual contrast between Pipelines and Runs #1610
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
feat: Improve visual contrast between Pipelines and Runs #1610
Conversation
**Changes:** * Removes grid when viewing a read-only run * Adds a background color to the Runs view that is different from the Pipelines view * Adds breadcrumbs that intelligently detects if a pipeline exists locally and gives the user the capability of navigating to the pipeline that triggered the run
6307c8c to
9f556af
Compare
|
CC: @Mbeaulne This is a change I have had sitting on my local since my first week. It stems from some confusion I and others have been experiencing about whether they are viewing a run or a pipeline - where people are currently relying on looking at which sidebars they are seeing, or certain symbols attached to the nodes. |
|
Would love to hear your thoughts! If you like it, we can try tweaking it after the other visual improvements you are planning |
|
Disregard the icon being in the title bar on one page but not the other. That is weird and I will fix that if we move forward |
camielvs
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
I agree with @camielvs. With the grey background we lose a little too much detail, such as the dotted background, which I think helps our users align components. As for the breadcrumb there is a bit of a bug: 1: go to run with a subgraph Screen Recording 2026-01-14 at 1.42.09 PM.mov (uploaded via Graphite) I think we should fix that bug and revert the background change, and then this PR should be good to go. Personally, I think this adds too much UI (the grey background or outline), and I don’t think we need additional UI to tell users whether they’re in a run or a subgraph. |
| useEffect(() => { | ||
| if (isCopied) { | ||
| const timer = setTimeout(() => { | ||
| setIsCopied(false); | ||
| }, 1500); | ||
| return () => clearTimeout(timer); | ||
| } | ||
| }, [isCopied]); | ||
|
|
||
| if (!pipelineName) { | ||
| return null; | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have a component CopyText.tsx that should be able to work for this .
| <BreadcrumbSeparator> | ||
| <ChevronRight className={cn("w-4 h-4", mutedTextClass)} /> | ||
| </BreadcrumbSeparator> | ||
| <BreadcrumbItem> | ||
| {runId ? ( | ||
| <div | ||
| className="group flex items-center gap-1 cursor-pointer" | ||
| onClick={handleCopyRunId} | ||
| onMouseEnter={() => setIsHovered(true)} | ||
| onMouseLeave={() => setIsHovered(false)} | ||
| title={`Click to copy: ${runId}`} | ||
| > | ||
| <span | ||
| className={cn( | ||
| "text-sm font-medium transition-colors duration-150", | ||
| isCopied ? "text-emerald-500" : textColorClass, | ||
| )} | ||
| > | ||
| Run {runId} | ||
| </span> | ||
| <span className="relative h-3.5 w-3.5"> | ||
| <Check | ||
| className={cn( | ||
| "absolute inset-0 h-3.5 w-3.5 text-emerald-500 transition-all duration-200", | ||
| isCopied | ||
| ? "rotate-0 scale-100 opacity-100" | ||
| : "-rotate-90 scale-0 opacity-0", | ||
| )} | ||
| /> | ||
| <Copy | ||
| className={cn( | ||
| "absolute inset-0 h-3.5 w-3.5 transition-all duration-200", | ||
| mutedTextClass, | ||
| isHovered && !isCopied | ||
| ? "rotate-0 scale-100 opacity-100" | ||
| : "rotate-90 scale-0 opacity-0", | ||
| )} | ||
| /> | ||
| </span> | ||
| </div> | ||
| ) : ( | ||
| <span className={cn("text-sm font-medium", textColorClass)}> | ||
| Run | ||
| </span> | ||
| )} | ||
| </BreadcrumbItem> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code could be simplified to this (and remove the copy logic above)
{runId && (
<>
<BreadcrumbSeparator>
<ChevronRight className={cn("w-4 h-4", mutedTextClass)} />
</BreadcrumbSeparator>
<BreadcrumbItem>
<CopyText alwaysShowButton className={textColorClass}>
{runId}
</CopyText>
</BreadcrumbItem>
</>
)}
| {/* Show breadcrumbs on run pages, otherwise show simple title */} | ||
| {isRunPage ? ( | ||
| <PipelineRunBreadcrumbs variant="topbar" /> | ||
| ) : ( | ||
| title && ( | ||
| <CopyText className="text-white text-md font-bold truncate max-w-32 sm:max-w-48 md:max-w-64 lg:max-w-md"> | ||
| {title} | ||
| </CopyText> | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we just remove this logic and use
<PipelineRunBreadcrumbs variant="topbar" />
seems to work and the logic is all in one place. Thoughts?


Description
Added breadcrumbs navigation to the Pipeline Run page, allowing users to navigate between pipeline runs and the editor. The breadcrumbs show the pipeline name and run ID, with the ability to:
Also updated the flow canvas background from the previous
<Background>component to a simple gray background color.Type of Change
Checklist
Screenshots (if applicable)
runs-contrast.mov (uploaded via Graphite)
Test Instructions